Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

refactor(project): improve typescript next/branch #62

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

@suddenlyGiovanni
Copy link

@suddenlyGiovanni suddenlyGiovanni commented Feb 1, 2021

As for PR: https://github.com/kentcdodds/react-hooks/pull/112#issue-554331129

Story

As a trainee, I want the reference files ./src/final-ts/** to be written in TypeScript React so that I can better understand React's API."

AC:

  1. Project should be configured to support both TypeScript and Javascript
  2. The trainee must be able to work in the ./src/exercises folder with no consequences.
  3. All source code in ./src/final-ts/should be refactored to TypeScript.
  4. Only syntax changes are allowed.
    4.1. The codebase should remain as close as possible to @kentcdodds's initial implementation.
    4.2. Should avoid distracting the user with complicated type signatures
  5. The code changes should expose only the best practices for production-ready React/Typescript codebase.
  6. All tests must pass!

refactor(project): to ts: initial commit
refactor(exercise-01): to ts
refactor(exercise-02): to ts
refactor(exercise-03): to ts
refactor(exercise-04): to ts
refactor(exercise-04): to ts: extra credit 01: prop getters
refactor(exercise-05): to ts
refactor(exercise-05): to ts: extra credit 01: default state reducer
refactor(exercise-05): to ts: extra credit 02: state reducer action types
refactor(exercise-06): to ts
refactor(exercise-06): to ts: extra credit 01: add read only warning
refactor(exercise-06): to ts: extra credit 02: add a controlled state warning
refactor(exercise-06): to ts: extra credit 03: extract warnings to a custom hook
refactor(exercise-06): to ts: extra credit 04: don’t warn in production

refactor(project): to ts: initial commit
refactor(exercise-01): to ts
refactor(exercise-02): to ts
refactor(exercise-03): to ts
refactor(exercise-04): to ts
refactor(exercise-04): to ts: extra credit 01: prop getters
refactor(exercise-05): to ts
refactor(exercise-05): to ts: extra credit 01: default state reducer
refactor(exercise-05): to ts: extra credit 02: state reducer action types
refactor(exercise-06): to ts
refactor(exercise-06): to ts: extra credit 01: add read only warning
refactor(exercise-06): to ts: extra credit 02: add a controlled state warning
refactor(exercise-06): to ts: extra credit 03: extract warnings to a custom hook
refactor(exercise-06): to ts: extra credit 04: don’t warn in production
<>
{React.Children.map(children, child =>
React.isValidElement(child) && typeof child.type !== 'string'
? React.cloneElement(child, {on, toggle})
Copy link
Contributor

@Aprillion Aprillion Mar 14, 2021
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was just thinking about how this exercise should look in TypeScript and there are 2 ways that made the most sense to me:

  • a) if we want ToggleOn, ToggleOff, and ToggleButton to be "exported" with the optional props, then this line needs to be something like React.cloneElement(child, {on, toggle, ...child.props}) to make the type signatures truthful
  • b) if we don't want to accept on and toggle props (similar to the original version), then we need different types for the "exported" component than the actual implementation

Copy link
Member

@kentcdodds kentcdodds Mar 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, when I get around to this exercise, I think I'm going to do something like this: https://twitter.com/markdalgleish/status/1370951759284162565

Aprillion reacted with rocket emoji
Copy link
Member

Thanks again for this PR. I've finished my own changes to the next branch now :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kentcdodds kentcdodds kentcdodds left review comments

+1 more reviewer

@Aprillion Aprillion Aprillion left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /